feat(js): use buffer instead of text-decoder #170

berendsliedrecht · 2023-08-17T18:25:58Z

Do not depend on fast-decoding anymore
added simple example app which can be used for testing.
- NOTE: it is currently rather annoying to fully recompile aries-askar for all the android and iOS architectures so we would have to find a solution for that.

This is by no means a perfect app for testing and if we would rather wait for a better testing app I can remove it from this PR and create it later.

Ideally I would like to move to use pnpm as it makes dealing with workspaces a whole lot easier. And create some way to "quickly" (still need to compile askar like 6 times...) use custom askar builds.

andrewwhitehead · 2023-08-17T22:14:20Z

Looks like the workflow needs to switch to node 18

TimoGlastra · 2023-08-18T09:39:13Z

wrappers/javascript/aries-askar-react-native/native/aries_askar.node

@@ -1,4 +0,0 @@
-# This is a placeholder file to prevent node-pre-gyp from installing the
-# aries-askar binary when cloning this repository. It won't be published to


This file should not be removed I think?

Otherwise when you run yarn install it will automatically download the binaries, which you may not want

For the React Native Example to be used without building it locally it is required now. It is not optimal, agreed.

I think it'd fail if we increase the native binary version. As the CI will then try to install the native binary, but it doesn't exist yet

Being required to build it locally doesn't seem like a bad thing to me, is there a reason we don't want to do that?

The main reason right now is that cross only allows to be ran on a Linux host which would make building the application for Android on macOS or Windows impossible. With a better example app we could add something where you specify the platform and you can choose to only build the application for iOS which would make it work on macOS.

andrewwhitehead · 2023-08-23T17:17:48Z

I'm trying to test this out but haven't gotten very far yet. A few things I've noticed (trying the ios build):

Commands like yarn build from the root directory produce an error, as they aren't defined by the app package
I had to add "uuid": "3.4.0" to the package dependencies to work around this issue: SDK45: Unable to resolve module uuid/v5 in expo go expo/expo#17261
I had to remove "@types/react" from aries-askar-react-native as it was producing duplicate type definitions

At the moment I'm stuck on this error. I think I need to build the iOS framework and put it somewhere, maybe under /ios?

Edit: It looks like I can bypass this issue by running yarn ios instead of yarn expo > open iOS simulator, and then I was able to run the application.

berendsliedrecht · 2023-08-31T07:49:07Z

I'm trying to test this out but haven't gotten very far yet. A few things I've noticed (trying the ios build):

Commands like yarn build from the root directory produce an error, as they aren't defined by the app package

I had to add "uuid": "3.4.0" to the package dependencies to work around this issue: SDK45: Unable to resolve module uuid/v5 in expo go expo/expo#17261

I had to remove "@types/react" from aries-askar-react-native as it was producing duplicate type definitions

At the moment I'm stuck on this error. I think I need to build the iOS framework and put it somewhere, maybe under /ios?
Edit: It looks like I can bypass this issue by running `yarn ios` instead of `yarn expo > open iOS simulator`, and then I was able to run the application.

Hmmm, maybe I should not have added the example app just yet. Setting it up where it is easy to change the askar libraries and build it will require some more effort.

I think for now the best we can do is just to remove the example app and create a PR later with it which will include a simple script which can build the libraries and place them in the correct location.

Signed-off-by: Berend Sliedrecht <[email protected]>

berendsliedrecht · 2023-09-05T19:18:05Z

I removed the example application as it was not ready to be merged. Will need to do some research for how we can properly set this up.

Signed-off-by: Berend Sliedrecht <[email protected]>

berendsliedrecht · 2023-09-06T07:33:19Z

@genaris any idea what is going on here? https://github.com/hyperledger/aries-askar/actions/runs/6089025819/job/16534903694 seems to refer to your patched version of the library.

genaris · 2023-09-06T10:31:32Z

@genaris any idea what is going on here? https://github.com/hyperledger/aries-askar/actions/runs/6089025819/job/16534903694 seems to refer to your patched version of the library.

I think there is an issue when yarn tries to resolve types for @2060.io/ref-napi (direct dependency) and types for ref-napi (it seems that @types/ref-array-di is requiring it). Both link to the same (as we use "@types/2060.io__ref-napi": "npm:@types/ref-napi")
so yarn complains with ... is trying to unpack in the same destination "xxx" as pattern ["xxx"]. This could result in a non deterministic behavior, skipping..

In this PR, the entry for @types/ref-napi in yarn.lock was removed. If I manually re-add it, it works. Not sure yet what's the right approach here to force these types before this breaks again. Maybe we'll need to patch @types/ref-array-di as well or create an actual @types/2060.io__ref-napi in DefinitelyTyped repo?

genaris · 2023-09-06T13:13:19Z

@genaris any idea what is going on here? https://github.com/hyperledger/aries-askar/actions/runs/6089025819/job/16534903694 seems to refer to your patched version of the library.

Update on this: I've added type definitions to the packages in order to attempt types clashing. Changes in nodejs' package.json would look like this:

I've tested it locally (build, check-types, test, etc.) from a clean checkout and seems to work fine. Hope this can help to unblock this PR!

Signed-off-by: Berend Sliedrecht <[email protected]>

feat(js): use buffer instead of text-decoder

berendsliedrecht requested review from andrewwhitehead and TimoGlastra August 17, 2023 18:25

berendsliedrecht changed the title ~~feat: use buffer instead of text-decoder~~ feat(js): use buffer instead of text-decoder Aug 17, 2023

berendsliedrecht force-pushed the use-buffer branch from c0311e2 to f9b8666 Compare August 17, 2023 18:26

berendsliedrecht mentioned this pull request Aug 17, 2023

Add profile management and store export to JS wrapper #169

Merged

TimoGlastra previously approved these changes Aug 18, 2023

View reviewed changes

genaris mentioned this pull request Sep 5, 2023

Migrate from SQLite to Postgres with Askar openwallet-foundation/credo-ts#1569

Open

berendsliedrecht dismissed TimoGlastra’s stale review via 3842c16 September 5, 2023 19:12

berendsliedrecht force-pushed the use-buffer branch 2 times, most recently from 3842c16 to 9fef5c4 Compare September 5, 2023 19:13

feat: use buffer instead of text-decoder

6033fa9

Signed-off-by: Berend Sliedrecht <[email protected]>

berendsliedrecht force-pushed the use-buffer branch from 9fef5c4 to 6033fa9 Compare September 5, 2023 19:16

berendsliedrecht requested a review from TimoGlastra September 5, 2023 19:18

berendsliedrecht mentioned this pull request Sep 5, 2023

Update dependencies #176

Merged

update rustc version in CI

1931e8f

Signed-off-by: Berend Sliedrecht <[email protected]>

TimoGlastra previously approved these changes Sep 6, 2023

View reviewed changes

berendsliedrecht dismissed TimoGlastra’s stale review via 6aa2141 September 6, 2023 07:38

TimoGlastra previously approved these changes Sep 6, 2023

View reviewed changes

berendsliedrecht dismissed TimoGlastra’s stale review via a6a022c September 6, 2023 13:35

berendsliedrecht force-pushed the use-buffer branch from 6aa2141 to a6a022c Compare September 6, 2023 13:35

lock rust version for CI Android

6c6f73c

Signed-off-by: Berend Sliedrecht <[email protected]>

berendsliedrecht force-pushed the use-buffer branch from a6a022c to 6c6f73c Compare September 7, 2023 08:15

berendsliedrecht requested a review from TimoGlastra September 7, 2023 10:02

TimoGlastra approved these changes Sep 7, 2023

View reviewed changes

berendsliedrecht merged commit ee46c20 into hyperledger:main Sep 7, 2023
26 checks passed

berendsliedrecht deleted the use-buffer branch September 7, 2023 11:36

This was referenced Sep 7, 2023

fix(js): bump patched ref-napi/ffi-napi hyperledger/anoncreds-rs#243

Merged

fix(js): bump patched ref-napi/ffi-napi hyperledger/indy-vdr#214

Merged

jamshale pushed a commit to jamshale/aries-askar that referenced this pull request Aug 18, 2024

Merge pull request hyperledger#170 from berendsliedrecht/use-buffer

c07b5ad

feat(js): use buffer instead of text-decoder

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(js): use buffer instead of text-decoder #170

feat(js): use buffer instead of text-decoder #170

berendsliedrecht commented Aug 17, 2023 •

edited

Loading

andrewwhitehead commented Aug 17, 2023

TimoGlastra Aug 18, 2023

berendsliedrecht Aug 18, 2023

TimoGlastra Aug 18, 2023

andrewwhitehead Aug 24, 2023

berendsliedrecht Aug 31, 2023

andrewwhitehead commented Aug 23, 2023 •

edited

Loading

berendsliedrecht commented Aug 31, 2023

berendsliedrecht commented Sep 5, 2023

berendsliedrecht commented Sep 6, 2023

genaris commented Sep 6, 2023 •

edited

Loading

genaris commented Sep 6, 2023

		@@ -1,4 +0,0 @@
		# This is a placeholder file to prevent node-pre-gyp from installing the
		# aries-askar binary when cloning this repository. It won't be published to

feat(js): use buffer instead of text-decoder #170

feat(js): use buffer instead of text-decoder #170

Conversation

berendsliedrecht commented Aug 17, 2023 • edited Loading

andrewwhitehead commented Aug 17, 2023

TimoGlastra Aug 18, 2023

Choose a reason for hiding this comment

berendsliedrecht Aug 18, 2023

Choose a reason for hiding this comment

TimoGlastra Aug 18, 2023

Choose a reason for hiding this comment

andrewwhitehead Aug 24, 2023

Choose a reason for hiding this comment

berendsliedrecht Aug 31, 2023

Choose a reason for hiding this comment

andrewwhitehead commented Aug 23, 2023 • edited Loading

berendsliedrecht commented Aug 31, 2023

berendsliedrecht commented Sep 5, 2023

berendsliedrecht commented Sep 6, 2023

genaris commented Sep 6, 2023 • edited Loading

genaris commented Sep 6, 2023

berendsliedrecht commented Aug 17, 2023 •

edited

Loading

andrewwhitehead commented Aug 23, 2023 •

edited

Loading

genaris commented Sep 6, 2023 •

edited

Loading